Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resync URL loading system error codes #4861

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dirtyhenry
Copy link

As I was developing the https://github.com/dirtyhenry/swift-blocks package, I noticed that some error codes from the URL Loading System error codes were out of sync (or missing) from what I could see in Xcode and on the Foundation documentation.

I hope it helps. 🙏

These error codes from the values from the docs. For instance,
https://developer.apple.com/documentation/foundation/1508628-url_loading_system_error_codes/nsurlerrorsecureconnectionfailed/
mentions `-1200`, not `-1201`.
It was missing from the code even though it is marked as done in
`Docs/API Surface.tasks`.
@dirtyhenry
Copy link
Author

I would love to hear your thoughts on this pull request.
If there's anything I missed or any concerns you have, I'm all ears!

Thanks.

@compnerd
Copy link
Member

This would be an ABI break, I'm not sure that we should be doing this ... why do these values have to match?

@dirtyhenry
Copy link
Author

Easy answer first :)

Why do these values have to match?

To make error management consistent no matter what OS is running the code. I believe this is very much in line with the overall objective of the project ie:

Our primary goal is to achieve implementation parity with Foundation on Apple platforms. This will help to enable the overall Swift goal of portability.

This would be an ABI break

I don't know how big of an impact this can be. Is an ABI break a deal-breaker for any PR? Or does it mean it has to wait a major version change to be taken into account?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants